Skip to content

Conversation

@Fznamznon
Copy link
Contributor

When a single #embed directive is used to initialize a char array, the case is optimized via swap of EmbedExpr to underlying StringLiteral, resulting in better performance in AST consumers.
While browsing through the code, I realized that
7122b70
which changed type of EmbedExpr made the "fast path" unreachable. This patch fixes this unfortunate situation.

When a single #embed directive is used to initialize a char array, the
case is optimized via swap of EmbedExpr to underlying StringLiteral,
resulting in better performance in AST consumers.
While browsing through the code, I realized that
7122b70
which changed type of EmbedExpr made the "fast path" unreachable. This
patch fixes this unfortunate situation.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

When a single #embed directive is used to initialize a char array, the case is optimized via swap of EmbedExpr to underlying StringLiteral, resulting in better performance in AST consumers.
While browsing through the code, I realized that
7122b70
which changed type of EmbedExpr made the "fast path" unreachable. This patch fixes this unfortunate situation.


Full diff: https://github.com/llvm/llvm-project/pull/121479.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaInit.cpp (+2-7)
  • (modified) clang/test/Analysis/embed.c (+1-1)
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 5909457b04e663..0dd5f468cf60bf 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -2030,13 +2030,8 @@ canInitializeArrayWithEmbedDataString(ArrayRef<Expr *> ExprList,
 
   if (InitType->isArrayType()) {
     const ArrayType *InitArrayType = InitType->getAsArrayTypeUnsafe();
-    QualType InitElementTy = InitArrayType->getElementType();
-    QualType EmbedExprElementTy = EE->getDataStringLiteral()->getType();
-    const bool TypesMatch =
-        Context.typesAreCompatible(InitElementTy, EmbedExprElementTy) ||
-        (InitElementTy->isCharType() && EmbedExprElementTy->isCharType());
-    if (TypesMatch)
-      return true;
+    StringLiteral *SL = EE->getDataStringLiteral();
+    return IsStringInit(SL, InitArrayType, Context) == SIF_None;
   }
   return false;
 }
diff --git a/clang/test/Analysis/embed.c b/clang/test/Analysis/embed.c
index 32f6c130325740..db8c270fb35de4 100644
--- a/clang/test/Analysis/embed.c
+++ b/clang/test/Analysis/embed.c
@@ -8,5 +8,5 @@ int main() {
         #embed "embed.c"
     };
     clang_analyzer_dump_ptr(SelfBytes); // expected-warning {{&Element{SelfBytes,0 S64b,unsigned char}}}
-    clang_analyzer_dump(SelfBytes[0]); // expected-warning {{Unknown}} FIXME: This should be the `/` character.
+    clang_analyzer_dump(SelfBytes[0]); // expected-warning {{47 U8b}}
 }

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Fznamznon Fznamznon merged commit cdad183 into llvm:main Jan 3, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants